From 8f12a041f3032ffde46c5460cdeeaaa8c1472493 Mon Sep 17 00:00:00 2001 From: "Karl O. Pinc" Date: Sat, 17 Nov 2018 01:53:21 -0600 Subject: [PATCH] Validation of session.secret setting --- README.rst | 5 +++ examples/development.ini | 7 ++++ examples/pgwui.ini | 10 ++++++ src/pgwui_server/__init__.py | 41 +++++++++++++++++++++++ tests/test___init__.py | 65 +++++++++++++++++++++++++++++++++++- 5 files changed, 127 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 5db39bf..1d33b23 100644 --- a/README.rst +++ b/README.rst @@ -248,6 +248,11 @@ installed in the virtual environment:: pgwui_venv/bin/pip install pyramid_debugtoolbar +Rudimentary validation of the ``pyramid_beaker`` ``session.secret`` +setting's value may be turned off. To do this change the +``pgwui.validate_hmac`` setting to ``False``. Having validation off +in production is not recommended. + Complete Documentation ---------------------- diff --git a/examples/development.ini b/examples/development.ini index 923cf50..07f896b 100644 --- a/examples/development.ini +++ b/examples/development.ini @@ -63,6 +63,13 @@ pgwui.dry_run = False # logout = /logout # upload = /upload +# Settings validation + +# Whether or not to validate the session.secret setting. +# session.secret must be valid to detect Cross-Site Request Forgery (CSRF) +# vulnerabilties. Validation is on by default. +pgwui.validate_hmac = False + # # Pyramid configuration diff --git a/examples/pgwui.ini b/examples/pgwui.ini index 4e8b870..7d00b3d 100644 --- a/examples/pgwui.ini +++ b/examples/pgwui.ini @@ -61,6 +61,16 @@ pgwui.dry_run = False # logout = /logmeout # upload = /put-in + + +# Settings validation + +# Whether or not to validate the session.secret setting. +# session.secret must be valid to detect Cross-Site Request Forgery (CSRF) +# vulnerabilties. Validation is on by default. +# pgwui.validate_hmac = True + + # # Pyramid configuration # https://docs.pylonsproject.org/projects/pyramid/en/latest/narr/environment.html diff --git a/src/pgwui_server/__init__.py b/src/pgwui_server/__init__.py index a0758e9..eb8ce80 100644 --- a/src/pgwui_server/__init__.py +++ b/src/pgwui_server/__init__.py @@ -22,6 +22,7 @@ '''Provide a way to configure PGWUI. ''' +from ast import literal_eval from pyramid.config import Configurator # Constants @@ -34,8 +35,12 @@ SETTINGS = set( 'dry_run', 'route_prefix', 'routes', + 'validate_hmac', ]) +# Required length of HMAC value +HMAC_LEN = 40 + # Exceptions class Error(Exception): @@ -49,6 +54,22 @@ class UnknownSettingKeyError(Error): super().__init__('Unknown PGWUI setting: {}'.format(key)) +class BadHMACError(Error): + pass + + +class NoHMACError(BadHMACError): + def __init__(self): + super().__init__('Missing session.secret configuration') + + +class HMACLengthError(BadHMACError): + def __init__(self): + super().__init__( + 'The session.secret value is not {} characters in length' + .format(HMAC_LEN)) + + # Functions def abort_on_bad_setting(key): @@ -59,11 +80,31 @@ def abort_on_bad_setting(key): raise UnknownSettingKeyError(key) +def do_validate_hmac(settings): + '''True unless the user has specificly rejected hmac validation + ''' + return ('pgwui.validate_hmac' not in settings + or literal_eval(settings['pgwui.validate_hmac'])) + + +def validate_hmac(settings): + '''Unless otherwise requested, validate the session.secret setting''' + if not do_validate_hmac(settings): + return + + if 'session.secret' not in settings: + raise NoHMACError() + + if len(settings['session.secret']) != HMAC_LEN: + raise HMACLengthError() + + def validate_settings(settings): '''Be sure all settings validate ''' for key in settings.keys(): abort_on_bad_setting(key) + validate_hmac(settings) def parse_assignments(lines): diff --git a/tests/test___init__.py b/tests/test___init__.py index 6bc9aa1..09aa885 100644 --- a/tests/test___init__.py +++ b/tests/test___init__.py @@ -23,6 +23,13 @@ import pytest import pgwui_server.__init__ as pgwui_server_init +# Constants + +TEST_SETTINGS = { + 'pgwui.validate_hmac': 'False', +} + + # Use contextlib.AbstractContextManager for Python >= 3.6 class MockConfigurator(): def __init__(self, **kwargs): @@ -69,6 +76,60 @@ def test_abort_on_bad_setting_good(): pgwui_server_init.abort_on_bad_setting('pgwui.pg_host') +# do_validate_hmac() + +def test_do_validate_hmac_none(): + '''pgwui.validate_hmac defaults to True''' + assert pgwui_server_init.do_validate_hmac({}) is True + + +def test_do_validate_hmac_True(): + '''Require hmac validation when pgwui.validate_hmac is True''' + result = pgwui_server_init.do_validate_hmac( + {'pgwui.validate_hmac': 'True'}) + assert result is True + + +def test_do_validate_hmac_False(): + '''No hmac validation when pgwui.validate_hmac is False''' + result = pgwui_server_init.do_validate_hmac( + {'pgwui.validate_hmac': 'False'}) + assert result is False + + +# validate_hmac() + +def test_validate_hmac_unvalidated(monkeypatch): + '''No error is raised when hmac validation is off''' + monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', + lambda *args: False) + pgwui_server_init.validate_hmac({}) + + +def test_validate_hmac_success(monkeypatch): + '''No error is raised when hmac is validated an the right length''' + monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', + lambda *args: True) + pgwui_server_init.validate_hmac( + {'session.secret': 'x' * pgwui_server_init.HMAC_LEN}) + + +def test_validate_hmac_missing(monkeypatch): + '''Raise error when hmac is validated and missing''' + monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', + lambda *args: True) + with pytest.raises(pgwui_server_init.NoHMACError): + pgwui_server_init.validate_hmac({}) + + +def test_validate_hmac_length(monkeypatch): + '''Raise error when hmac is validated and the wrong length''' + monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', + lambda *args: True) + with pytest.raises(pgwui_server_init.HMACLengthError): + pgwui_server_init.validate_hmac({'session.secret': ''}) + + # validate_settings() def test_validate_settings(monkeypatch): @@ -82,6 +143,8 @@ def test_validate_settings(monkeypatch): monkeypatch.setattr(pgwui_server_init, 'abort_on_bad_setting', mock_abort_on_bad_setting) + monkeypatch.setattr(pgwui_server_init, 'validate_hmac', + lambda *args: None) settings = {'key1': 'value1', 'key2': 'value2'} @@ -165,7 +228,7 @@ def test_main(monkeypatch): # Integration tests def test_main_integrated(): '''Does not raise errors or warnings''' - pgwui_server_init.main({}) + pgwui_server_init.main({}, **TEST_SETTINGS) # Functional tests -- 2.34.1